-
Notifications
You must be signed in to change notification settings - Fork 92
gw-gravity-forms-rename-uploaded-files.php: Fixed an issue where renamed multi-file uploads in Nested Forms became malformed after editing the child entry.
#1160
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
…named multi-file uploads in Nested Forms became malformed after editing the child entry.
WalkthroughUpdate to Changes
Sequence Diagram(s)sequenceDiagram
participant U as User
participant GF as Gravity Forms
participant PL as Plugin (rename_uploaded_files)
participant FS as Filesystem
participant GM as GF Meta
U->>GF: Submit form with file upload
GF->>PL: Invoke rename_uploaded_files(entry, form)
PL->>FS: Attempt rename file
alt rename succeeds
FS-->>PL: Return renamed path
PL->>PL: Build $renamed_url
PL->>GF: GF_Field_FileUpload.get_upload_root_info(form.id)
GF-->>PL: Return {root_path, root_url}
PL->>GM: gform_update_meta(entry_id, meta_key_hash($renamed_url), {path, url, file_name})
GM-->>PL: Meta updated
PL-->>GF: Append $renamed_url to results
else rename fails
FS-->>PL: Error
PL->>PL: Append original file path to results
end
PL-->>GF: Return updated file URLs list
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested reviewers
Pre-merge checks (2 passed, 1 warning)❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
…named multi-file uploads in Nested Forms became malformed after editing the child entry.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (2)
gravity-forms/gw-gravity-forms-rename-uploaded-files.php (2)
101-114: Solid fix syncing GF file-path meta; add guards and only sync on successful rename.
Great approach writing the path meta keyed by the renamed URL. Two robustness tweaks:
- Only proceed if rename() succeeded.
- Guard calls for older GF versions; skip meta sync if methods aren’t available.
- Optional: remove stale meta for the original URL to avoid orphaned keys.
Apply within this hunk:
+ if ( ! $result ) { + $renamed_files[] = $_file; + continue; + } + - $renamed_url = $this->get_url_by_path( $renamed_file, $form['id'] ); - $renamed_files[] = $renamed_url; + $renamed_url = $this->get_url_by_path( $renamed_file, $form['id'] ); + $renamed_files[] = $renamed_url; - // Keep Gravity Forms file-path meta in sync with the renamed URL. - $root = GF_Field_FileUpload::get_upload_root_info( $form['id'] ); - gform_update_meta( - $entry['id'], - GF_Field_FileUpload::get_file_upload_path_meta_key_hash( $renamed_url ), - array( - 'path' => trailingslashit( $root['path'] ), - 'url' => trailingslashit( $root['url'] ), - 'file_name' => wp_basename( $renamed_url ), - ) - ); + // Keep Gravity Forms file-path meta in sync with the renamed URL (GF 2.5+). + if ( + class_exists( 'GF_Field_FileUpload' ) + && method_exists( 'GF_Field_FileUpload', 'get_upload_root_info' ) + && method_exists( 'GF_Field_FileUpload', 'get_file_upload_path_meta_key_hash' ) + ) { + $root = GF_Field_FileUpload::get_upload_root_info( $form['id'] ); + gform_update_meta( + $entry['id'], + GF_Field_FileUpload::get_file_upload_path_meta_key_hash( $renamed_url ), + array( + 'path' => trailingslashit( $root['path'] ), + 'url' => trailingslashit( $root['url'] ), + 'file_name' => wp_basename( $renamed_url ), + ) + ); + // Optional: clean up stale meta for the old URL. + if ( function_exists( 'gform_delete_meta' ) ) { + gform_delete_meta( $entry['id'], GF_Field_FileUpload::get_file_upload_path_meta_key_hash( $_file ) ); + } + }Quick checks to verify:
- Edit a Nested Form child entry that has multi-file uploads; confirm file names remain correct.
- Spot-check on a site with older GF (<2.5) if you support it; ensure no fatal due to missing methods.
117-117: Fix typo in comment.
Change “nfo images” → “no images”.- // In cases where 3rd party add-ons offload the image to a remote location, nfo images can be renamed. + // In cases where 3rd party add-ons offload the image to a remote location, no images can be renamed.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
gravity-forms/gw-gravity-forms-rename-uploaded-files.php(2 hunks)
🔇 Additional comments (1)
gravity-forms/gw-gravity-forms-rename-uploaded-files.php (1)
16-16: Version bump to 2.7 — looks good.
No concerns from me on the header update.
saifsultanc
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks good to me. I do agree with the coderabbit suggestion of the safety check for Only proceed if rename() succeeded.
Can you please encorporate that?
…named multi-file uploads in Nested Forms became malformed after editing the child entry.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
🧹 Nitpick comments (2)
gravity-forms/gw-gravity-forms-rename-uploaded-files.php (2)
16-16: Version bump noted — sync docs/changelog.
Please update any visible snippet headers/README/changelog to reflect 2.7 and reference the Nested Forms edit fix.
101-104: Good fallback on rename failure; add debug logging.
Appending the original URL prevents data loss. Consider lightweight logging to aid support triage.- if ( ! $result ) { - $renamed_files[] = $_file; - continue; - } + if ( ! $result ) { + $renamed_files[] = $_file; + if ( defined( 'WP_DEBUG' ) && WP_DEBUG ) { + error_log( sprintf( + 'GW_Rename_Uploaded_Files: rename() failed for entry %d: %s -> %s', + $entry['id'], + $file, + $renamed_file + ) ); + } + continue; + }
saifsultanc
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM!
Context
⛑️ Ticket(s): https://secure.helpscout.net/conversation/3049464511/88344?viewId=8172236
Summary
Issue: When users used the Rename Uploaded Files snippet on a multi-file upload field inside a Nested Form, filenames were correctly renamed when adding the child entry, but if the user then edited the child entry and saved, the filename became malformed.
This happened because GF's stored path metadata referenced the original URL instead of the URL of our renamed file. On edit, GF couldn't find the path info for the renamed URL and fell back to deriving a filename from the full URL, which after sanitization produced the long incorrect basename.
Fix: Immediately after each file is renamed, we now also write GF's file path meta for the new URL.
Quick before and after: https://www.loom.com/share/297e7afb604c4e9788ca24eb451b8fcf